Skip to content

Conversation

@shentongmartin
Copy link
Contributor

@shentongmartin shentongmartin commented Jan 4, 2026

ADK Agent Handler Refactoring

Problems

This PR addresses two related problems in the ADK agent architecture:

Problem 1: AgentMiddleware Struct Cannot Be Extended by Users

AgentMiddleware is a struct with optional callback fields:

type AgentMiddleware struct {
    AdditionalInstruction string
    AdditionalTools       []tool.BaseTool
    BeforeChatModel       func(context.Context, *ChatModelAgentState) error
    AfterChatModel        func(context.Context, *ChatModelAgentState) error
    WrapToolCall          compose.ToolMiddleware
}

Core limitation: Struct types are closed; users cannot add new methods.

Consider this scenario: a user wants to implement a MemoryMiddleware that coordinates between BeforeChatModel and AfterChatModel, and needs a custom method GetMemoryStats() to expose internal statistics:

// User wants to do this, but can't:
type MemoryMiddleware struct {
    AgentMiddleware  // Embed base struct
    memory *MemoryStore
    stats  *MemoryStats
}

// ❌ Cannot add new methods - struct type is closed
// Framework only recognizes AgentMiddleware's fixed fields, cannot call user-added methods
func (m *MemoryMiddleware) GetMemoryStats() *MemoryStats {
    return m.stats
}

// ❌ Even if user defines this type, framework cannot use it:
// config.Middlewares is []AgentMiddleware, not []MemoryMiddleware

Other limitations:

  1. Callbacks cannot return modified context - Callbacks only return error, cannot inject values into context for downstream use.

  2. Mixed definition styles - AgentMiddleware mixes direct field assignment (AdditionalInstruction) with function fields (BeforeChatModel), inconsistent style.

  3. Shared configuration scattered in closures - Multiple callback functions capture shared configuration via closures, configuration is scattered:

// AgentMiddleware approach: configuration captured via closures, scattered across anonymous functions
func NewTokenLimitMiddleware(counter *TokenCounter, threshold int, placeholder string) adk.AgentMiddleware {
    return adk.AgentMiddleware{
        BeforeChatModel: func(ctx context.Context, state *adk.ChatModelAgentState) error {
            // Access counter, threshold, placeholder via closure
            if counter.Count(state.Messages) > threshold {
                state.Messages = counter.Truncate(state.Messages, threshold/2, placeholder)
            }
            return nil
        },
        AfterChatModel: func(ctx context.Context, state *adk.ChatModelAgentState) error {
            // Also access counter via closure - configuration scattered across closures
            log.Printf("final token count: %d", counter.Count(state.Messages))
            return nil
        },
    }
}

// HandlerMiddleware approach: configuration as struct fields, centralized
type TokenLimitHandler struct {
    *BaseHandlerMiddleware
    counter     *TokenCounter
    threshold   int
    placeholder string  // Configuration centralized in struct definition, clear at a glance
}

Problem 2: Callback-Based Event Sending Timing Is Uncontrollable

Events (model responses, tool results) are sent via callbacks registered on the graph:

// Old approach: callbacks triggered by ChatModel/ToolsNode components
callbacks := genReactCallbacks(cbHandler)
chain.WithCallbacks(callbacks)

// Event sending in callback handler
func (h *cbHandler) onGraphEnd(ctx context.Context, info *callbacks.RunInfo, output any) {
    h.generator.Send(EventFromMessage(output.(*schema.Message), nil, schema.Assistant, ""))
}

Core problem: Callback timing is fixed by components, cannot send events after wrapper processing.

Expected event sending timing:
Model.Generate() → UserWrapper.Transform(result) → Send event(transformed)
                                                    ↑ After wrapper processing

Actual callback timing:
Model.Generate() → Callback triggers → Send event(original) → UserWrapper.Transform(result)
                   ↑ Triggers immediately when component returns, wrapper hasn't processed yet

Secondary problem: Callback handlers are passed via context, parent-child agents share by default.

// Parent agent registers callback
ctx = callbacks.InitCallbacks(ctx, handler)

// Child agent inherits same handler
childAgent.Run(ctx, input)  // Child agent's events also sent to parent handler

// Need manual address filtering to avoid duplicate events
func (h *cbHandler) onGraphEnd(ctx context.Context, info *callbacks.RunInfo, output any) {
    if info.RunPath != expectedPath {  // Manual filtering
        return
    }
    h.generator.Send(...)
}

Solutions

Solution for Sub-problem 1: HandlerMiddleware Interface

Introduce interface-based HandlerMiddleware to complement existing AgentMiddleware:

type HandlerMiddleware interface {
    BeforeAgent(ctx context.Context, agentCtx *AgentContext) (context.Context, *AgentContext, error)
    BeforeModelRewriteHistory(ctx context.Context, messages []Message) (context.Context, []Message, error)
    AfterModelRewriteHistory(ctx context.Context, messages []Message) (context.Context, []Message, error)
    WrapTool(ctx context.Context, t tool.BaseTool) (tool.BaseTool, error)
    WrapModel(ctx context.Context, m model.BaseChatModel) (model.BaseChatModel, error)
}

Key design choices:

  1. Interface over struct - Users implement custom handlers with arbitrary internal state; interfaces are open.
  2. Context propagation - All methods return (context.Context, ..., error) to carry context through the lifecycle.
  3. Direct wrapping APIs - WrapTool/WrapModel directly return wrapped components, aligning with tool.BaseTool and model.BaseChatModel interfaces.
  4. BaseHandlerMiddleware defaults - Embed *BaseHandlerMiddleware to inherit no-op methods and allow reflection-based override detection.

Why not split into many interfaces? These five methods are coordinated extension points in one lifecycle; a unified interface keeps usage simple while defaults cover omissions.

Solution for Sub-problem 2: Wrapper-Based Event Sending (after user wrappers)

Replace callback-based event sending with an explicit wrapper chain where events are emitted after user processing:

Model wrapper order (outer → inner):

  1. agentMiddlewareModelWrapper (append input/output messages, run Before/AfterChatModel)
  2. historyRewriteModelWrapper (Before/AfterModelRewriteHistory; After sees input+response)
  3. eventSenderModelWrapper (emit transformed results after user wrappers)
  4. Handlers' WrapModel (registered in reverse so earlier handlers sit outermost)
  5. callbackInjectionModelWrapper (only if the model lacks callback support)

This ordering makes event timing controllable, isolates parent/child agents (no context-passed handlers), and keeps composition explicit and testable.


Design Decisions

Decision 1: Keep AgentMiddleware

Considered: Remove AgentMiddleware entirely, migrate all users to AgentHandler.

Rejected because: Backward compatibility. Existing users have extensively used AgentMiddleware, forced migration would be a breaking change.

Decision: Keep both coexisting. AgentMiddleware is suitable for simple static additions, AgentHandler is suitable for complex scenarios requiring extensibility.

Decision 2: Return Wrapped Components via WrapTool/WrapModel

Initial design: Wrapper interfaces with next functions (ToolCallWrapper, ModelCallWrapper).

Problems: Extra middle types and indirection; users must learn next calling conventions; inconsistent with core interfaces.

Final design: WrapTool(ctx, t) (tool.BaseTool, error) and WrapModel(ctx, m) (model.BaseChatModel, error) directly return wrapped components.

Decision 3: Move State from Context to State Struct

Before: Runtime values stored in context:

ctx = context.WithValue(ctx, runtimeReturnDirectlyKey{}, returnDirectly)

Problems:

  1. Unfriendly for interrupt/resume - Values in context are lost during serialization, state incomplete after resume
  2. Implicit passing - Any point in call chain can read/write these values, hard to trace
  3. Extra context layer overhead - Each WithValue creates a new context layer

After: Runtime values in State struct:

type State struct {
    // ...
    RuntimeReturnDirectly map[string]struct{}
    ReturnDirectlyEvent   *AgentEvent
    generator             *AsyncGenerator[*AgentEvent]
    retryAttempt          int
}

Trade-off: Using State requires access via compose.ProcessState, introducing extra synchronization overhead (mutex lock/unlock). We accept this overhead in exchange for better interrupt/resume support and more explicit state management.


Execution Order

Agent.Run(input)
    │
    ├─► Middlewares applied (static: instruction, tools)
    │
    ├─► Handlers.BeforeAgent (once per run)
    │   - Modify instruction, tools, returnDirectly
    │
    └─► Loop: Model → Tools → Model → ...
        │
        ├─► Handlers.BeforeModelRewriteHistory
        │   - Inject context, modify messages before model call
        │
        ├─► Model call (wrapped by ModelCallWrapper chain)
        │   - eventSenderModelWrapper sends model response event
        │   - User wrappers can intercept/transform
        │
        ├─► Handlers.AfterModelRewriteHistory
        │   - Process response, save state
        │
        └─► Tool calls (wrapped by ToolCallWrapper chain)
            - toolResultEventSenderWrapper sends tool result event
            - User wrappers can intercept/transform

Middleware execution order (outermost to innermost):

  1. toolResultEventSenderWrapper (internal - sends tool result events)
  2. Handlers' embedded ToolCallWrapper (in registration order)
  3. Middlewares' WrapToolCall (in registration order)
  4. User-provided ToolsConfig.ToolCallMiddlewares

Summary

Problem Solution
AgentMiddleware struct cannot be extended by users (cannot add new methods) HandlerMiddleware interface + BaseHandlerMiddleware defaults
Callbacks cannot return modified context All handler methods return (context.Context, ..., error)
Mixed definition styles (direct fields + function fields) HandlerMiddleware unified as methods
Callback event sending timing fixed, cannot send after wrapper processing Wrapper chain controls event sending timing
Callbacks passed via context, parent-child agents share causing duplicate events Wrappers are part of agent instance, naturally isolated

Usage Guide

When to Use AgentMiddleware

// Simple, static additions
agent, _ := NewChatModelAgent(ChatModelAgentConfig{
    Middlewares: []AgentMiddleware{{
        AdditionalInstruction: "Always be polite.",
        AdditionalTools:       []tool.BaseTool{myTool},
    }},
})

When to Use HandlerMiddleware

// Dynamic behavior, context modification, call wrapping
type MemoryHandler struct {
    *BaseHandlerMiddleware
    memory *MemoryStore
}

func (h *MemoryHandler) BeforeAgent(ctx context.Context, agentCtx *AgentContext) (context.Context, *AgentContext, error) {
    agentCtx.Tools = append(agentCtx.Tools, h.memory.RetrievalTool())
    return ctx, agentCtx, nil
}

func (h *MemoryHandler) BeforeModelRewriteHistory(ctx context.Context, messages []Message) (context.Context, []Message, error) {
    memories := h.memory.Retrieve(ctx, messages)
    return ctx, append(memories, messages...), nil
}

agent, _ := NewChatModelAgent(ChatModelAgentConfig{
    Handlers: []HandlerMiddleware{&MemoryHandler{memory: myMemory}},
})

Design Decision: No Helper Functions

We chose not to provide helpers like WithInstruction/WithTools. They duplicate AgentMiddleware capabilities, add cognitive overhead, and can be easily hand-written when truly needed.

Decision 4: BaseHandlerMiddleware Uses Pointer Receiver + Cached Reflection

Use pointer receivers on BaseHandlerMiddleware so method overrides can be detected via reflection at construction time; cache the detection to eliminate runtime reflection overhead.

Decision 5: Name It HandlerMiddleware (not AgentHandler)

Prefer concise naming aligned with package context (like http.Handler). HandlerMiddleware is unambiguous and avoids conflict with existing AgentMiddleware.

Decision 6: Move Before/AfterModelRewriteHistory from Graph into Model Wrapper

Core reason: Graph modelPreHandle/modelPostHandle cannot propagate context.Context, violating the design of Before/AfterModelRewriteHistory. Moving into wrappers ensures context chaining and removes state duplication and timing inconsistencies.

@codecov
Copy link

codecov bot commented Jan 4, 2026

Codecov Report

❌ Patch coverage is 83.05598% with 112 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.19%. Comparing base (39eabc0) to head (ec65a64).

Files with missing lines Patch % Lines
adk/chatmodel.go 82.20% 35 Missing and 20 partials ⚠️
adk/wrappers.go 79.40% 31 Missing and 24 partials ⚠️
adk/handler.go 92.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #660      +/-   ##
==========================================
- Coverage   81.32%   81.19%   -0.14%     
==========================================
  Files         124      126       +2     
  Lines       11921    12119     +198     
==========================================
+ Hits         9695     9840     +145     
- Misses       1503     1539      +36     
- Partials      723      740      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shentongmartin shentongmartin force-pushed the refactor/agent_middleware branch from 5298ecd to a5a144c Compare January 4, 2026 06:50
@shentongmartin shentongmartin force-pushed the refactor/agent_middleware branch 3 times, most recently from f1909ee to 30d5e3e Compare January 12, 2026 13:03
Change-Id: I9a84f2a3e13721d06a86f5f2739772831067f10b
- Rename BeforeChatModelHandler to MessageStatePreProcessor
- Rename AfterChatModelHandler to MessageStatePostProcessor
- Rename BeforeChatModel/AfterChatModel methods to PreProcessMessageState/PostProcessMessageState
- Remove type aliases InvokableToolCallNext/StreamableToolCallNext (inline function types)
- Update helper functions: WithBeforeChatModel -> WithMessageStatePreProcessor, WithAfterChatModel -> WithMessageStatePostProcessor
- Update all references in chatmodel.go and react.go
- Fix state.Messages assignment in no-tools case to maintain backward compatibility

The new naming better conveys that message changes are persisted to agent state
and visible to subsequent iterations of the agent loop.

Change-Id: I0d7be02aeae82a9412a5615685ce7e6fb88d517c
…modification

Key changes:
- Move BeforeAgent callback from NewChatModelAgent to Run/Resume time
- Add support for runtime tools modification with lazy graph rebuilding
- Simplify AgentHandler interface with unified method signatures
- Add runtime ReturnDirectly configuration via context
- Support graph config compatibility checking to minimize rebuilds

Implementation details:
- Add graphConfig struct to track graph configuration (hasTools, hasReturnDirectly)
- Add preAppliedBeforeAgentData to avoid duplicate BeforeAgent calls
- Implement getRunFunc with lazy rebuild logic for incompatible configs
- Support runtime ReturnDirectly via context in react.go
- Update handler helpers to use new AgentConfig-based API

Breaking changes:
- AgentHandler interface methods now take AgentConfig instead of separate params
- BeforeAgent errors now returned at Run time instead of NewChatModelAgent time

Change-Id: Ic392eacba8f2c5acdc9df32f1d0e64e54711a900
…s in HandlerToolsConfig

ToToolMetas and ToolMetasToHandlerToolsConfig now accept context.Context
and return error when tool.Info() fails, instead of silently skipping
the ReturnDirectly configuration.

Change-Id: I850c29a4588c18a11b66b9171403314c420f3dcf
…tions

Change-Id: Icc84f849f03ca5bacbcdf7c6841d927c726eccd9
- Remove Name() method from AgentHandler interface (unused)
- Replace WrapInvokableToolCall/WrapStreamableToolCall with GetToolMiddleware()
- Use compose.ToolMiddleware directly for tool wrapping
- Simplify BaseAgentHandler (remove name field)
- Replace WithInvokableToolWrapper/WithStreamableToolWrapper with WithToolMiddleware
- Add documentation for ToolInput field mutability

This change:
1. Reduces interface complexity by using existing compose.ToolMiddleware
2. Makes it easier to wrap both invoke and stream tool calls together
3. Provides forward compatibility for future tool types

Change-Id: I0a5247795abe954326f1eb633e079c54a0126560
- Add type aliases: ToolCall, ToolResult, StreamToolResult (from compose package)
- Add ToolCallHandler interface with HandleInvoke/HandleStream methods
- Add BaseToolCallHandler with pass-through implementations
- Replace GetToolMiddleware() with GetToolCallHandler() in AgentHandler
- Replace WithToolMiddleware with WithToolCallHandler helper
- Add legacyToolMiddlewareAdapter for AgentMiddleware backward compatibility
- Update collectToolMiddlewares to convert ToolCallHandler to compose.ToolMiddleware

This change:
1. Combines invoke/stream handling into one interface (must implement both)
2. Avoids Middleware/Endpoint terminology
3. Uses clearer type names (ToolCall vs ToolInput)
4. Explicit next() signature: next(ctx, call) makes data flow clear

Change-Id: I9b3918d9f491b611e31606f83cef8c48f5233f4a
- Rename AgentConfig to AgentRunContext with only Instruction and Tools fields
- Remove unused AgentRunOptions struct and AgentConfig.Input field
- Remove unused HandlerToolsConfig and related functions
- Move ToolMeta and AgentRunContext to handler.go
- Delete tools_config.go
- Remove AgentRunOptions parameter from applyBeforeAgent

This simplifies the handler interface by focusing only on instruction and tools
configuration, which are the primary customization points for handlers.

Change-Id: I4c4182e4f2708ecd9c2d6cda7ff71a04020f4676
…reAgent signature

- Rename ToolCallHandler to ToolCallWrapper
- Rename HandleInvoke/HandleStream to WrapInvoke/WrapStream
- Rename BaseToolCallHandler to BaseToolCallWrapper
- Rename GetToolCallHandler() to GetToolCallWrapper()
- Rename WithToolCallHandler() to WithToolCallWrapper()
- Change BeforeAgent signature to return *AgentRunContext for functional style

The new signature: BeforeAgent(ctx, runCtx) -> (ctx, runCtx, error)
This is consistent with BeforeModelRewriteHistory and AfterModelRewriteHistory,
making the data flow explicit and avoiding in-place mutation.

Change-Id: Ice503ed130c3bbd07ef93cc7baa92ac6812dc568
- Add RuntimeReturnDirectly field to State struct
- Add ReactInput struct with Messages and RuntimeReturnDirectly fields
- Change react graph input type from []Message to *ReactInput
- Add init node to transfer input's RuntimeReturnDirectly to State
- Update toolPreHandle to read from st.RuntimeReturnDirectly instead of context
- Remove supportRuntimeReturnDirectly from reactConfig
- Remove context-based getRuntimeReturnDirectlyFromCtx/setRuntimeReturnDirectlyToCtx
- Simplify graphConfig by removing hasReturnDirectly field
- Always include return directly branch in graph (since runtime config is always possible)

This change:
1. Makes data flow explicit: runtime config enters via input, stored in state
2. Supports interrupt/resume: state is checkpointed, context is not
3. Removes confusing compile-time flag that controlled runtime behavior

Change-Id: I024f2fbcb8bfd1f260217eddb7f1c0c3d2e3a283
- Remove AgentHandler interface methods from AgentMiddleware struct
- Remove legacyToolMiddlewareAdapter (no longer needed)
- Add middlewares field to ChatModelAgent struct
- Update NewChatModelAgent to store middlewares separately from handlers
- Update applyBeforeAgent to apply middleware AdditionalInstruction and AdditionalTools
- Add collectToolMiddlewaresFromMiddlewares function for direct middleware extraction
- Update reactConfig to include middlewares field
- Apply middleware BeforeChatModel/AfterChatModel in both:
  - react graph model pre/post handlers
  - buildNoToolsRunFunc model pre/post handlers

This change allows AgentMiddleware and AgentHandler to evolve independently:
- AgentHandler can add new methods without affecting AgentMiddleware
- AgentMiddleware remains a simple configuration struct
- No boilerplate adapter code needed

Change-Id: Ia591863b878d9b1e891357dc3239982eab23d257
ChatModelAgent.Resume was using buildRunFunc directly, which returns
the default cached graph. However, if the initial Run used a temporary
graph due to dynamic tools added by BeforeAgent handlers, Resume would
use the wrong graph topology.

Changed Resume to use getRunFunc which:
1. Applies BeforeAgent handlers to get runtime tools
2. Checks if default graph is compatible with runtime config
3. Builds temporary graph if needed

This ensures Resume uses the same graph topology as the initial Run.

Change-Id: I8b3209c40652bbf84e32f9c27893f5af814303d9
Middleware AdditionalInstruction and AdditionalTools are static values
known at NewChatModelAgent time. Previously, applyBeforeAgent iterated
through all middlewares on every request to collect these values.

Changes:
- Add middlewareInstruction and middlewareTools fields to ChatModelAgent
- Pre-compute these values in NewChatModelAgent
- Simplify applyBeforeAgent to use pre-computed values

This optimization avoids per-request iteration through middlewares for
static values, while handlers still process dynamically per request.

Change-Id: I82a489c67389b3e2d3a36a793afdfe84d7496d5f
Add test case 'DynamicToolCanBeCalledByModel' that verifies:
1. A handler can dynamically add a tool via WithTools
2. The model can call that dynamically added tool
3. The tool is actually executed (verified via callback)

Also add 'callableTool' test helper that supports an invokeFn callback
for verifying tool execution.

Change-Id: I0aaa359618cf2f4c444a8b5e400f82e3400d528d
…ision

buildRunFunc was only considering ToolsConfig.Tools when deciding whether
to build a tools-enabled graph. It ignored middlewareTools (from
AgentMiddleware.AdditionalTools), which are known at instantiation time.

This caused unnecessary temporary graph builds when:
- ToolsConfig.Tools is empty
- But middlewareTools is not empty

Changes:
- Include middlewareTools when computing initialTools
- Change condition from len(bc.toolsNodeConf.Tools) to len(initialTools)

This ensures the default graph accounts for middleware tools, reducing
the need for temporary graph builds at runtime.

Change-Id: Iadd7112c80126efbd9ac72b6997c077afb4499bd
…figuration

Restore the conditional check that only adds the return-directly part of
the graph when there are tools configured with ReturnDirectly: true.
This simplifies the graph topology in the common case.

Changes:
- Add hasReturnDirectly field to graphConfig
- Update computeGraphConfig to compute hasReturnDirectly from tools
- Update isGraphConfigCompatible to check hasReturnDirectly compatibility
- Add hasReturnDirectly field to reactConfig
- Conditionally add return-directly branch in newReact
- Update buildReactRunFunc to accept hasReturnDirectly parameter
- Update tests to set hasReturnDirectly when testing return-directly

When no tools have ReturnDirectly: true, the graph now has a simple edge
from toolNode to chatModel instead of the return-directly branch logic.

If runtime tools change hasReturnDirectly from false to true, a temporary
graph with the return-directly branch will be built.

Change-Id: Ie88d091a6083376213e3571bd58565e269e96f91
Consolidate build preparation logic by moving initialTools computation
from buildRunFunc into prepareBuildContext.

Changes:
- Add initialTools field to buildContext struct
- Move initialTools computation (from toolsNodeConf.Tools + middlewareTools)
  into prepareBuildContext
- Simplify buildRunFunc to use bc.initialTools directly

This improves code organization by keeping all build preparation logic
in one place (prepareBuildContext), making buildRunFunc simpler and
more focused on its core responsibility.

Change-Id: Iac3921c058669ed16ed440c2293ef1320c3caab7
Remove middlewareInstruction and middlewareTools fields from ChatModelAgent
as they were redundant - just derived values from the middlewares field.

Changes:
- Remove middlewareInstruction and middlewareTools from ChatModelAgent struct
- Remove pre-computation loop from NewChatModelAgent
- Move middleware instruction/tools computation into prepareBuildContext
- Update applyBeforeAgent to take buildContext parameter and use bc.baseInstruction
  and bc.initialTools directly
- Update all applyBeforeAgent call sites
- Restructure getRunFunc to call prepareBuildContext before applyBeforeAgent

This eliminates redundancy and ensures single source of truth (middlewares field)
for middleware configuration.

Change-Id: Ied4836650bbad4bbe80c39deb0615ca66bb0a0bc
… handling

- Improve code formatting in buildNoToolsRunFunc
- Use state.Messages directly instead of creating temporary variables
- Fix post-handler to return the last message from state.Messages
  (in case handlers modified the messages)
- Add error check for empty messages after post-handler

Change-Id: I79a548873b20341bde13df56ffab37c5a240e5aa
prepareBuildContext was called on every request in getRunFunc, but all
inputs are immutable after the first run begins. This was wasteful.

Changes:
- Move graphConfig into buildContext struct (they are correlated)
- Replace graphConfig field with buildContext field in ChatModelAgent
- Update prepareBuildContext to compute graphConfig
- Cache buildContext in buildRunFunc (via sync.Once)
- Update getRunFunc to use cached a.buildContext
- Change isGraphConfigCompatible to a function taking two configs

Now prepareBuildContext is called only once per agent lifetime, and
the result is cached in ChatModelAgent.buildContext for reuse.

Change-Id: I356f68ca2b9b7fe073752a6ef546eb5ab96d16ed
When there are no handlers, getRunFunc was still calling applyBeforeAgent,
setPreAppliedBeforeAgentToCtx, computeGraphConfig, and isGraphConfigCompatible
unnecessarily.

Add a fast path that returns the default run func immediately when
len(a.handlers) == 0, avoiding all the unnecessary processing.

Change-Id: I21e2c685d1886995eab8efd88d9f895bed6299d7
The graphConfig type only had two boolean fields (hasTools, hasReturnDirectly)
that were computed from initialTools. Since initialTools is already in
buildContext, it makes sense to have these derived fields directly in
buildContext rather than in a separate type.

Changes:
- Remove graphConfig type
- Add hasTools and hasReturnDirectly fields directly to buildContext
- Update prepareBuildContext to compute these fields inline
- Remove computeGraphConfig method
- Rename isGraphConfigCompatible to isRuntimeCompatible with simpler signature
- Update getRunFunc to compute runtime flags inline and use isRuntimeCompatible
- Update buildRunFunc to use bc.hasReturnDirectly directly

This simplifies the code by removing one type and making the relationship
between initialTools and the derived flags more explicit.

Change-Id: I8ba1ec3bce84317d5af1e4a01fca4b3a7400d09e
Remove hasTools and hasReturnDirectly fields from buildContext since they
can be easily derived from initialTools.

Changes:
- Remove hasTools and hasReturnDirectly from buildContext struct
- Add hasReturnDirectlyTool helper function
- Update isRuntimeCompatible to compute from bc.initialTools
- Update prepareBuildContext to remove derived fields computation
- Update buildRunFunc to use hasReturnDirectlyTool(bc.initialTools)
- Fix bug: make a copy of initialTools in applyBeforeAgent to prevent
  handlers from mutating the cached bc.initialTools

The copy fix is important because handlers can modify tools[i].ReturnDirectly
in-place, which would corrupt the cached buildContext and cause incorrect
graph compatibility checks.

Change-Id: I1ba41e1104062b31d22d1c10d416fe0e47ae44fb
These two small utility functions did very little and added indirection.
Inline their logic directly at the call sites for clearer code.

Changes:
- Remove hasReturnDirectlyTool function
- Remove isRuntimeCompatible function
- Inline hasReturnDirectly check in buildRunFunc
- Inline compatibility check in getRunFunc with clearer needTempGraph variable

Change-Id: I1f2175ebb43c69f0a48a9b4a20babbd662aa9964
…irectly, opts

Major refactoring to simplify runtime configuration handling:

1. Update runFunc signature to accept instruction, returnDirectly, and opts
   as parameters instead of reading from context

2. Remove preAppliedBeforeAgent mechanism entirely:
   - Delete preAppliedBeforeAgentCtxKey, preAppliedBeforeAgentData
   - Delete setPreAppliedBeforeAgentToCtx, getPreAppliedBeforeAgentFromCtx

3. Update applyBeforeAgent to return all needed values:
   - instruction, tools, toolInfos, returnDirectly, configChanges
   - Compute topology change flags (needToolsGraph, needReturnDirectlyBranch)

4. Update getRunFunc to return all values needed by caller:
   - runFunc, instruction, tools, toolInfos, returnDirectly, ctx, error

5. Update Run and Resume methods:
   - Get all values from getRunFunc
   - Add WithTools and WithToolList options for tool overrides
   - Pass instruction and returnDirectly to runFunc

6. Simplify buildNoToolsRunFunc and buildReactRunFunc:
   - Use passed instruction and returnDirectly directly
   - Remove context-based retrieval and redundant computation
   - Remove 're-add system tools' logic (user's responsibility)

Benefits:
- Clear data flow: everything is explicit parameters
- No context-passing mechanism
- runFunc is a clean template
- ~80 lines of code removed
- Much easier to understand and maintain

Change-Id: I52b7a7009e5274d31744ad2cccf629745104b959
… buildContext

1. Remove redundant hasReturnDirectly field from reactConfig
   - Replace with len(toolsReturnDirectly) > 0 check

2. Make reactInput fields unexported
   - messages, runtimeReturnDirectly (lowercase)

3. Expand buildContext to include runtime values
   - Add instruction, tools, toolInfos fields
   - Add needToolsGraph, needReturnDirectlyBranch flags
   - Remove separate configChanges type

4. Update applyBeforeAgent to modify buildContext in place
   - Returns (context.Context, error) instead of 7 values
   - All computed values stored in bc

5. Update getRunFunc to return *buildContext
   - Returns (runFunc, *buildContext, context.Context, error)
   - Caller accesses bc.instruction, bc.tools, etc.

6. Update Run and Resume methods
   - Use bc.toolInfos, bc.tools, bc.instruction, bc.returnDirectly

Benefits:
- Simpler reactConfig - no redundant boolean
- All runtime configuration in one place (buildContext)
- Cleaner applyBeforeAgent - single return value
- Easier to understand data flow

Change-Id: I1ad3f43391c2dece885632e7351e070fae817f8c
…of modifying in place

The previous approach modified buildContext in place, which is problematic:
1. buildContext is stored in a.buildContext and shared across all requests
2. When request-time handlers become a reality, modifying in place would cause race conditions
3. Added fields (instruction, tools, toolInfos) duplicated information

Changes:
1. Remove instruction and tools fields from buildContext
   - Reuse baseInstruction for runtime instruction
   - Reuse toolsNodeConf.Tools for runtime tools
   - Keep toolInfos (needed for WithTools option)

2. Update applyBeforeAgent to return new buildContext
   - Returns (context.Context, *buildContext, error)
   - Creates new buildContext with runtime values
   - Original buildContext remains immutable

3. Update getRunFunc
   - Creates runtimeBC for no-handlers case too
   - Uses runtimeBC from applyBeforeAgent

4. Update Run and Resume methods
   - Use bc.baseInstruction (runtime instruction)
   - Use bc.toolsNodeConf.Tools (runtime tools)

Benefits:
- buildContext in a.buildContext remains immutable
- Safe for future request-time handlers
- No duplicate fields
- Clear separation: initial config vs runtime config

Change-Id: I2a6bd3854e1ac629688689ffe1de5fd3dea1bafa
…in AgentRunContext

ToolMeta was introduced when we had separate handler types (ModifyTools).
Now with AgentRunContext, we can put tools and returnDirectly directly
in the context, which is cleaner and aligns with buildContext structure.

Changes:
1. Remove ToolMeta type from handler.go

2. Update AgentRunContext
   - Tools: []tool.BaseTool (was []ToolMeta)
   - ReturnDirectly: map[string]struct{} (new field)

3. Update applyBeforeAgent in chatmodel.go
   - Use slices.Clone for tools
   - Use copyMap for returnDirectly
   - Directly use runCtx.Tools and runCtx.ReturnDirectly

4. Update WithToolsFunc signature
   - fn(ctx, tools []tool.BaseTool, returnDirectly map[string]struct{})
     -> (ctx, []tool.BaseTool, map[string]struct{}, error)

5. Update WithTools helper
   - Simply append tools to runCtx.Tools

6. Update handler_test.go
   - Use new WithToolsFunc signature

Benefits:
- Simpler API - no intermediate ToolMeta type
- Aligns with buildContext structure
- Clearer intent - tools and returnDirectly are separate concerns
- Easier for users to manipulate tools in handlers

Change-Id: I1348c22f5366f8c54417b7e9e575a141bf5f61b8
1. Simplify buildContext fields
   - Combine needToolsGraph + needReturnDirectlyBranch into rebuildGraph
   - Add toolUpdated flag to track if tools were modified

2. Simplify applyBeforeAgent
   - Use genToolInfos helper instead of manual loop
   - Inline rebuildGraph condition

3. Simplify buildReactRunFunc
   - Remove hasReturnDirectly parameter
   - Use bc.returnDirectly directly

4. Reorder getRunFunc return values
   - (runFunc, *buildContext, context.Context, error)
   - -> (context.Context, runFunc, *buildContext, error)
   - More idiomatic Go - context first

5. Optimize Run and Resume
   - Only append tool options when bc.toolUpdated is true
   - Avoids unnecessary option processing when tools unchanged

Change-Id: Id3d0df6c576049ad93eaf7ba4ad5d3b70b96f23f
Move error and interrupt handling from callback-based approach to direct
handling in runFunc closures. This simplifies the architecture and provides
direct access to necessary context.

Changes:
1. Remove callback-based error handlers
   - Remove (*cbHandler).onGraphError (~36 lines)
   - Remove (*noToolsCbHandler).onGraphError (~6 lines)
   - Remove chain/graph error handlers from callback registration

2. Move interrupt handling to runFunc
   - buildNoToolsRunFunc: Add error sending directly
   - buildReactRunFunc: Add full interrupt handling logic inline
     - Extract interrupt info from error
     - Get checkpoint data from store
     - Create composite interrupt event
     - Send event to generator

3. Remove generator.Close() from runFunc
   - Caller (Run/Resume) handles closing in defer block

Benefits:
- Simpler architecture - no callback indirection for errors
- Direct access to store and agent name in interrupt handling
- Cleaner callbacks - only handle streaming/events
- Correct responsibility - caller closes generator

Change-Id: I3d5645aca33069da222931aad1c2886778dec464
…via State

- Add generator and returnDirectlyEvent fields to State
- Add generator field to reactInput and pass it through initLambda
- Create sendToolResultEvent and sendStreamToolResultEvent functions using ProcessState
- Remove context-based tool result senders (adkToolResultSender types and context keys)
- Remove reactGraphHandler callback (no longer needed)
- Combine generator and historyModifier into single state modifier in Resume
- Remove unused addrDepthChain constant

Change-Id: Ib0439f6c16d1baf806ad46c8a03975d8a2584b04
- Add toolPostHandle as StreamStatePostHandler to tools node in react.go
- Remove onToolsNodeEnd and onToolsNodeEndWithStreamOutput from cbHandler
- Remove sendReturnDirectlyToolEvent from cbHandler
- Remove toolsNodeHandler from genReactCallbacks
- Remove addrDepthToolsNode constant

The return directly event is now sent via State-based StreamStatePostHandler
instead of callback-based address depth checking.

Change-Id: Ib6ee380d0c75737dcaf2bbdeabad6102b66d43a8
…pattern

- Add ModelCallWrapper interface for wrapping model calls (similar to ToolCallWrapper)
- Add wrappedChatModel to chain multiple wrappers around a chat model
- Add eventSenderModelWrapper to send model result events via State
- Add toolResultEventSenderWrapper to send tool result events via State
- Move model wrapping and tool event sender setup to NewChatModelAgent
- Remove callback-based event sending (cbHandler, noToolsCbHandler, etc.)
- Update buildNoToolsRunFunc to use State instead of ChatModelAgentState
- Update retry_chatmodel to write retryAttempt to State via ProcessState
- Add tests for ModelCallWrapper functionality

Change-Id: Ib56cce21dbba302c8a766aaba84c94620cb1cd29
…nabled

- Add callbackInjectionWrapper that injects callbacks using public
  callbacks.OnStart, OnEnd, OnError, OnEndWithStreamOutput functions
- Modify newWrappedChatModel to append callback wrapper when inner
  model doesn't have callbacks enabled
- Update IsCallbacksEnabled() to always return true since wrappedChatModel
  now handles callbacks internally
- Add test cases to verify callback injection behavior:
  - simpleChatModelWithoutCallbacks: model without IsCallbacksEnabled
  - inputOutputModifyingWrapper: wrapper that modifies input/output
  - TestWrappedChatModel_CallbackInjection: verifies callbacks capture
    real model input/output (after wrapper input mod, before output mod)

Change-Id: I0577c32257ab104b2975149db7153e3c98e08e44
Move compose.NewChain creation from inside runFunc to the function body
of buildNoToolsRunFunc and buildReactRunFunc. This ensures the chain
structure is created once during agent build, and only compilation
happens per run.

Changes:
- buildNoToolsRunFunc: move noToolsInput type and chain creation outside
  runFunc, add instruction field to noToolsInput
- buildReactRunFunc: create reactRunInput wrapper type, move chain
  creation outside runFunc

This improves performance by avoiding repeated chain construction on
every agent run.

Change-Id: Iab9f0f72519465e796e8790a818e620b1ffdedee
When a return-directly tool completes and another tool interrupts
simultaneously, the ReturnDirectlyEvent was lost because it was stored
in an unexported field that wasn't serialized to checkpoint.

Changes:
- Export returnDirectlyEvent to ReturnDirectlyEvent in State struct
- Add Role and ToolName fields to MessageVariant serialization
- Add TestReturnDirectlyEventSentAfterResume regression test

This ensures the ReturnDirectlyEvent is persisted to checkpoint and
correctly sent after resume.

Change-Id: I5d1e31ee34fa9259157b71a90d1518e85e8c8a1c
…e wrappers

- Rename wrap_chatmodel.go to wrappers.go for a more general-purpose name
- Move popToolGenAction and toolResultEventSenderWrapper from react.go
  to wrappers.go to consolidate all wrapper-related code in one file

Change-Id: I2b9b5eb0c2b28ccb7a210511cf56ade93f2d5942
…tibility

Change-Id: I49d3dfc832e8f5e5f3c663d08090aba735079814
…nd handler.go

- Add StreamingToolWrappersPipeline subtest to verify WrapStream is called
- Modify TestReturnDirectlyEventSentAfterResume to cover bc.toolUpdated path in Resume
- Add TestGetComposeOptions to cover WithChatModelOptions and WithToolOptions

Change-Id: I3126e9a84234a7e1b55147a5bac806da448e1b75
…ntation

- Add handler type (%T) to BeforeAgent error message for better debugging
- Improve nil generator error messages with guidance on state initialization
- Document tool call middleware execution order in NewChatModelAgent

Change-Id: I4429a4616bba7836d7fe7ba8c70f0fc14b088e14
- Replace GetToolCallWrapper/GetModelCallWrapper getters with embedded interfaces
- Rename methods to avoid collision: WrapInvoke→WrapToolInvoke, WrapStream→WrapToolStream/WrapModelStream, WrapGenerate→WrapModelGenerate
- Update BaseAgentHandler to embed BaseToolCallWrapper and BaseModelCallWrapper
- Simplify collectModelWrappersFromHandlers since handlers now directly implement wrappers
- Add comprehensive design rationale documentation to AgentHandler interface

Change-Id: I3de461509dbb42d3c2588c21fe1f05d4ebc194dd
…iddleware

- Update AgentHandler interface comment explaining why interface over struct
- Update AgentMiddleware struct comment explaining limitations and when to use
- Update Handlers/Middlewares field comments in ChatModelAgentConfig

Change-Id: I387bd7ee74d5bbb58cca196d556676684b76a26c
@shentongmartin shentongmartin force-pushed the refactor/agent_middleware branch from c9b4133 to 65763e4 Compare January 13, 2026 09:17
…od on AgentHandler

- Remove ModelCall, ModelResult, StreamModelResult structs
- Remove ModelCallWrapper interface and BaseModelCallWrapper
- Add WrapModel method directly to AgentHandler interface
- Add WrapModelFunc type and update WithModelWrapper helper
- Simplify wrappers.go: remove wrappedChatModel, add buildWrappedModel
- Add baseChatModelAdapter to bridge BaseChatModel to ToolCallingChatModel
- Update tests to use function-based API

Change-Id: I725e67f3cf2451cec0f126d65d0e44753d832179
… on AgentHandler

- Remove ToolCall, ToolResult, StreamToolResult type aliases
- Remove ToolCallWrapper interface and BaseToolCallWrapper struct
- Add WrapTool method directly to AgentHandler interface
- Add wrapToolFuncToMiddleware to convert WrapTool to compose.ToolMiddleware
- Add endpointInvokableTool and endpointStreamableTool adapters
- Convert toolResultEventSenderMiddleware to wrapToolWithEventSender
- Remove WrapToolFunc and WrapModelFunc type aliases (use inline function types)
- Rename WithToolCallWrapper to WithToolWrapper
- Update tests to use function-based API

Change-Id: Ibe5b9debfb97771f5879b02f22f1656c1faffddd
- Change all BaseAgentHandler methods from value receiver to pointer receiver
- Update all embeddings from BaseAgentHandler to *BaseAgentHandler
- Enables reflection-based detection of method overrides

Change-Id: I362f33352b4a07d47e6ff563eb3a913ac71e38c0
- Rename AgentHandler interface to HandlerMiddleware
- Rename BaseAgentHandler struct to BaseHandlerMiddleware
- Makes it explicit that these types are middlewares
- Package context (adk) already implies agent-related

Change-Id: Idd47a5b070d9c5b01a303b76a538af6c5fde4bcd
- Add handlerInfo struct to cache method override detection at construction time
- Add isMethodOverridden() using reflection to compare function pointers
- Update ChatModelAgent.handlers from []HandlerMiddleware to []handlerInfo
- Update all runtime calls to check cached flags before calling handler methods
- Skip default BaseHandlerMiddleware methods at runtime with zero overhead

Performance: cached reflection has same overhead as direct call (~0.3ns),
while per-call reflection would cost ~450ns per call.

Change-Id: Id59b8ab251fa487ffac8d11a2f8ce3e1e1a557a0
Remove all helper functions from handler_helpers.go:
- WithInstruction, WithInstructionFunc
- WithTools, WithToolsFunc
- WithBeforeAgent
- WithBeforeModelRewriteHistory, WithAfterModelRewriteHistory
- WithToolWrapper, WithModelWrapper

Rationale:
1. WithInstruction/WithTools duplicate AgentMiddleware.AdditionalInstruction/AdditionalTools
2. Other helpers just wrap functions in structs - users can easily write their own
3. Reduces API surface and cognitive load (9 fewer exported functions)
4. Cleaner API: use AgentMiddleware for simple cases, custom HandlerMiddleware for complex cases

Change-Id: If3252fc4f69c7fd84a772acd42a2e6d3943b7d23
@shentongmartin shentongmartin force-pushed the refactor/agent_middleware branch from 45242b4 to c663b19 Compare January 13, 2026 13:33
Remove generateWithProxyCallbacks and streamWithProxyCallbacks methods.

These methods were unreachable because:
1. buildWrappedModel always wraps the model with callbackInjectionModelWrapper
   (if the model doesn't handle callbacks) before passing to retryChatModel
2. baseChatModelAdapter.IsCallbacksEnabled() always returns true
3. Therefore innerHandlesCallbacks is always true in retryChatModel

Also removed the innerHandlesCallbacks field and simplified the code.

Change-Id: I3d95d3fffbd4cb89c546cd524aa851de85e8d893
Changes:
- Apply HandlerMiddleware.WrapTool directly to tools at construction time
  (or after BeforeAgent if any handler has BeforeAgent)
- Convert eventSenderTool to eventSenderToolMiddleware (outermost middleware)
- Remove wrapToolFuncToMiddleware and virtual tool adapters (dead code)
- Add hasAnyBeforeAgent flag to skip redundant wrapping

Benefits:
- No per-request overhead: tools are wrapped once, not on every tool call
- Simpler code: removed ~100 lines of virtual tool adapter code
- Event sender now receives final modified results (after all wrappers)

Wrapping order (innermost to outermost):
1. Original Tool (+ compose's callback injection if needed)
2. HandlerMiddleware.WrapTool (applied directly, reverse handler order)
3. AgentMiddleware.WrapToolCall (middleware)
4. ToolsNodeConfig.ToolCallMiddlewares (middleware)
5. eventSenderToolMiddleware (sends events after all modifications)

Change-Id: I34cf26e02b9b17d6b91fb32afddcb12cd59ffc34
…tory to model wrapper

Move BeforeModelRewriteHistory and AfterModelRewriteHistory from graph
pre/post handlers to a model wrapper (historyRewriteModelWrapper).

Benefits:
- Context propagation: context changes from BeforeModelRewriteHistory
  now propagate to the model call and subsequent handlers
- Consistent ordering: all HandlerMiddleware methods now run in the
  model wrapper chain, separate from AgentMiddleware callbacks

Model wrapper order (innermost to outermost):
1. callbackInjectionModelWrapper (if model doesn't handle callbacks)
2. HandlerMiddleware.WrapModel (reverse handler order)
3. eventSenderModelWrapper
4. historyRewriteModelWrapper (BeforeModelRewriteHistory/AfterModelRewriteHistory)

Note: AgentMiddleware.BeforeChatModel/AfterChatModel still run in graph
pre/post handlers, outside the model wrapper chain.

Change-Id: I80b163e03ddc7d95416507929fd4b27edb0a0e6a
@shentongmartin shentongmartin force-pushed the refactor/agent_middleware branch from 3afd5ec to ec65a64 Compare January 14, 2026 08:31
type HandlerMiddleware interface {
// BeforeAgent is called before each agent run, allowing modification of
// the agent's instruction and tools configuration.
BeforeAgent(ctx context.Context, runCtx *AgentContext) (context.Context, *AgentContext, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agent 运行前切面要基于这个定义吗?但我看这里没有 agent input 相关信息

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你加上就行,我只加了之前有的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants